Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix unit tests #172

Merged
merged 8 commits into from
May 22, 2024
Merged

Fix unit tests #172

merged 8 commits into from
May 22, 2024

Conversation

swissspidy
Copy link
Member

@swissspidy swissspidy commented May 22, 2024 •
edited
Loading

Trying to fix tests after wp-cli/wp-cli-tests#206

I realized we were not actually running tests against PHP 5.3, only 5.6+, so I figured we can just bump the requirement while at it.

To-do:

  • Figure out why tests are failing on CI, the autoloader doesn't seem to be working anymore... 🤔 Any help appreciated!

@swissspidy swissspidy added this to the 0.12.0 milestone May 22, 2024
@swissspidy swissspidy marked this pull request as ready for review May 22, 2024 19:01
@swissspidy swissspidy requested a review from a team as a code owner May 22, 2024 19:01
@thelovekesh
Copy link
Member

thelovekesh commented May 22, 2024 •
edited
Loading

@swissspidy It seems like on local, tests autoloading has no effect. Tests are running on local even when I disable this line

php-cli-tools/tests/bootstrap.php

Line 22 in 6507ba2

spl_autoload_register( 'cli_autoload' );

@thelovekesh
Copy link
Member

Can I send some tests commits to this PR?

@swissspidy
Copy link
Member Author

Yes I noticed that too, but no idea why.

And the error message on CI don't seem to make sense either

PHP Warning:  Uncaught require(/home/runner/work/php-cli-tools/php-cli-tools/lib/cli/Table/Ascii.php): failed to open stream: No such file or directory

/home/runner/work/php-cli-tools/php-cli-tools/tests/bootstrap.php:19

/lib/cli/Table/Ascii.php does exist

@thelovekesh
Copy link
Member

Error message have Table in the path while the directory name is table.

@thelovekesh
Copy link
Member 

├── table
│   ├── Ascii.php
│   ├── Renderer.php
│   └── Tabular.php

@swissspidy
Copy link
Member Author

That was it! Thanks @thelovekesh!

@thelovekesh
Copy link
Member

Still a mystery to me, why it was not reproducible on my local 🤔. I guess it has something to do with tests custom autoloader.

@swissspidy
Copy link
Member Author

Case insensitive filesystem I suppose

@swissspidy swissspidy merged commit 2d1da47 into master May 22, 2024
17 checks passed
@swissspidy swissspidy deleted the fix/unit-tests branch May 22, 2024 20:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thelovekesh thelovekesh thelovekesh approved these changes

Assignees
No one assigned
Labels
scope:testing
Projects
None yet
Milestone
0.12.0
Development

Successfully merging this pull request may close these issues.
2 participants
@swissspidy @thelovekesh